media: Add RendererType::kUrlPlayer and buildflag - #10979
Conversation
🤖 Gemini Suggested Commit Message💡 Pro Tips for a Better Commit Message:
|
There was a problem hiding this comment.
Code Review
This pull request introduces the use_starboard_url_player build flag and kUrlPlayer renderer type to support URL-based player functionality, specifically for tvOS builds with Starboard media enabled. The feedback recommends un-nesting the USE_STARBOARD_URL_PLAYER preprocessor checks from USE_STARBOARD_MEDIA in the Mojo enum traits mapping to ensure consistency with the renderer definitions and prevent compilation issues if the flags are decoupled.
Add use_starboard_url_player GN capability flag, derived from use_starboard_media and tvOS platform detection. Expose it as BUILDFLAG(USE_STARBOARD_URL_PLAYER) through build_config.h and register matching Mojo enabled_features entry. Add RendererType::kUrlPlayer = 13 to the C++ enum, Mojom enum, traits mapping, and histogram. The new enum is gated behind USE_STARBOARD_URL_PLAYER so non-tvOS builds are unaffected. This establishes the capability boundary for all subsequent URL-player CLs. Bug: 512045535
|
cc : @rakuco @jkim-julie @Gyuyoung |
| if (is_ios) { | ||
| import("//build/config/apple/mobile_config.gni") | ||
| use_starboard_url_player = use_starboard_media && target_platform == "tvos" | ||
| } else { | ||
| use_starboard_url_player = false | ||
| } |
There was a problem hiding this comment.
I think just use_starboard_url_player = use_starboard_media && is_ios is fine. A comment describing the variable would be good too.
There was a problem hiding this comment.
If use_starboard_url_player = use_starboard_media && (is_ios || is_tvos), why not just use is_ios or is_tvos behind the code gated by use_starboard_media?
As both use_starboard_url_player and use_starboard_media are for Starboard Player, I think it's not ideal to introduce one more global build flag for a single Starboard platform.
There was a problem hiding this comment.
Thanks for the feedback. Let me walk through the reasoning.
use_starboard_url_player is not a platform-specific flag, it is a capability flag. Today this capability is limited to tvOS, but conceptually it represents "this build can delegate playback to a platform URL player."
At the Starboard API layer, we already have SB_HAS(PLAYER_WITH_URL), the Starboard layer already treats URL player as a distinct capability with its own flag. However, that flag is defined in platform headers (starboard/tvos/shared/configuration_public.h) and is not available in GN, Mojom IDL, or upstream Chromium C++ files that do not include Starboard headers. So above Starboard, we need another way to express the same capability.
There is no is_tvos flag in the build system:
Unlike is_ios, is_mac, or is_linux, there is no general GN is_tvos variable.
Why a single GN variable is required:
Mojom [EnableIf=...] accepts only a single feature name from GN. It cannot express compound conditions like use_starboard_media && is_ios && target_platform == "tvos". So we need a dedicated GN variable for the Mojom gate. Leaving the Mojom methods ungated ("always present") is not viable either as it forces every platform to implement URL player methods.
We need these Mojom methods because in the URL player architecture the complete player lives in the GPU process, and we need IPC to share player status (duration, buffering info, etc.) back to the renderer/JS side.
Consistency across layers(like GN/Starboard/mojom/non-starboard):
Since we need a GN variable for the Mojom gate anyway, reusing the same derived flag in C++ keeps the URL player condition tied to one named capability. This is not trying to replace every tvOS platform guard in the tree. The platform-specific code can still use the appropriate platform guard for its layer. The point is narrower: URL player is a specific media capability, and the code that depends on that capability should not have to reconstruct it differently in each layer.
Using [EnableIf=is_ios] would work mechanically for the current tvOS build, but it would make the Mojom method availability depend on the iOS platform bucket rather than on URL-player support. That is the wrong boundary for this IPC. The URL-player methods are needed because this build uses the URL-player architecture, where playback state has to cross process boundaries, not simply because the build is in the iOS family.
With a named capability flag, the same concept is available consistently above the Starboard layer:
- Starboard:
SB_HAS(PLAYER_WITH_URL) - Mojom:
[EnableIf=use_starboard_url_player] - Non-Starboard C++:
BUILDFLAG(USE_STARBOARD_URL_PLAYER)
The same reasoning applies on the C++ side. We could continue using the compound check (BUILDFLAG(USE_STARBOARD_MEDIA) && BUILDFLAG(IS_IOS_TVOS)), but that condition is broader than URL player because it is also true for the regular Starboard media path. As URL player diverges further from the HTML5 player path, keeping that distinction explicit becomes more important.
The flag is derived, not independent:
This is not a new user-facing configuration. It is derived in one place:
use_starboard_url_player = use_starboard_media && is_ios
It is a named alias for the compound condition.
Naming:
We agree the current name is confusing. The use_starboard_ prefix makes this look like a sibling of use_starboard_media, while the intent is to describe URL player capability. We are happy to rename it to something clearer, for example enable_url_player, enable_platform_url_player, or has_player_with_url.
If we settle on a clearer name, for example enable_url_player, the Starboard-side define(SB_HAS_PLAYER_WITH_URL) could also be aligned in a follow-up. That is not required for these CLs stack, but it would make the capability naming more consistent across layers.
There was a problem hiding this comment.
Where is the place that tvOS need to delegate playbacks to SbPlayer using URL player? I haven't checked tvOS code yet, but I think there should be a way for tvOS to build it without additional GLOBAL build flags.
There was a problem hiding this comment.
The tvOS-specific delegation happens when HLS playback is initialized with a URL instead of demuxed media buffers.
Regular Starboard playback uses Chromium's media pipeline: Chromium fetches/demuxes the media and sends encoded buffers to Starboard through SbPlayerWriteSamples(). In that path Chromium owns most of the pipeline state.
The tvOS URL-player path is different. For HLS, tvOS needs to delegate playback to Apple's
AVPlayer. This native player accepts only URL of media file to play. In our Starboard layer that is exposed through the URL-player APIs in starboard/tvos/shared/media/url_player.h: SbUrlPlayerCreate, SbUrlPlayerSetDrmSystem, and SbUrlPlayerGetExtraInfo. Our main responsibility is to send player url from chromium world to starboard world where this player lives but using chromium media pipeline.
In the Chromium/Cobalt side, UrlPlayerRendererWrapper::InitializeWithUrl() carries the URL
into the GPU-side renderer. That reaches SbPlayerBridge::CreateUrlPlayer(), which calls
sbplayer_interface_->CreateUrlPlayer(...); the default implementation delegates to
SbUrlPlayerCreate().
Because AVPlayer owns manifest fetching, demuxing, buffering, decoding, and rendering in this
path, Chromium's normal demuxer/buffer pipeline is bypassed. But JS still needs media state such
as duration, buffered ranges, current time, ended/error, and encrypted init data. That is why
the URL-player path adds IPC/Mojom surface such as InitializeWithUrl() to send URL from renderer to GPU and player status updates( like duration, buffered ranges) from GPU to renderer for status plumbing.
I do not intend this to be an independent global knob. The flag is derived from the same tvOS/Starboard condition you mentioned. The reason it exists as a named build feature is that the affected code is above the Starboard layer: shared media/Mojo/Blink files do not include Starboard platform headers, so they cannot
use SB_HAS(PLAYER_WITH_URL). Mojom also cannot directly reference compound condition.; it needs
GN to expose a named feature for [EnableIf=...].
Once that named feature exists for Mojom, using the same flag consistently in GN and C++ means the URL-player capability is identified the same way at every layer above Starboard, rather than each layer reconstructing it from a different combination of platform and media flags.So this is effectively the "URL player capability" equivalent of SB_HAS(PLAYER_WITH_URL) for the layers above Starboard.
There was a problem hiding this comment.
The comments are splits to many PRs, but refer to https://github.com/youtube/cobalt/pull/11066/changes#r3501515960.
|
Thanks for all review comments. We are moving ahead with approach in PR #10698 (UrlPlayer using StarboardRenderer). |
Add
use_starboard_url_playerGN capability flag, derived fromuse_starboard_mediaand tvOS platform detection. Expose it asBUILDFLAG(USE_STARBOARD_URL_PLAYER)through build_config.h andregister matching Mojo enabled_features entry.
Add RendererType::kUrlPlayer = 13 to the C++ enum, Mojom enum,
traits mapping, and histogram. The new enum is gated behind
USE_STARBOARD_URL_PLAYERso non-tvOS builds are unaffected.This establishes the capability boundary for all subsequent
URL-player CLs.
Verified build success using below command :
ninja -C out/tvos-arm64-simulator_qa cobalt nplb media_unittests
Bug: 512045535